refactor(daemon): decouple extension activation refresh - #10991

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

E2E test report

Validated locally on macOS with Node.js 22.

• Activation contract: global/workspace, singular/batch, legacy/V2, and override clear operations commit without directly calling session refresh; terminal results omit refresh counts.
• Failure isolation: pending and rejected runtime-refresh mocks do not delay or downgrade activation success.
• Explicit application: the workspace refresh operation remains independent and advances the applied generation after success.
• Eventual convergence: the unchanged 30-second generation reconciler still detects and applies pending generations.
• Web Shell: a daemon advertising extension_activation_explicit_refresh receives a current-workspace refresh submission after activation; the page unlocks without polling that refresh. Older daemons receive no duplicate refresh, and submission failure does not roll back activation.
• Regression coverage: Extension controller and route suites confirm install/update/uninstall and Extension-owned Skill reconciliation remain intact.

Commands/results: Extension route/controller/capability tests 67 passed; Web Shell behavior tests 4 passed; full daemon server file 1,173 passed with 3 unrelated concurrent socket/timeout flakes, all 3 passing on serial rerun; npm run build, npm run typecheck, focused ESLint, and git diff --check passed.

Not run against a production instance; no production Extension state was changed.

Thanks for the PR!

Template looks good ✓ — all required headings present, Tested-on filled in, full Chinese translation.

Problem: partly substantiated, and one half of the stated motivation doesn't survive reading the code.

The latency coupling is real. Activation currently runs refreshExtensionsForAllSessions across every registered runtime inside the operation, serialized through the concurrency-1 runtimeReconciliationQueue, and the codebase already carries a RECONCILE_SLOW_MS warning path for exactly this. Pulling that work off a policy file write is a legitimate goal. There's no measurement behind it though — "makes the activation operation slow" has no before/after number.

The second claim, that coupling "lets refresh failures downgrade an otherwise successful policy commit", isn't what the code does. All three refresh-failure paths already land on succeeded_with_warnings, never failed: the multi-runtime Promise.all catches rejections into a rejected result, the single-workspace path is wrapped in catch (refreshErr), and the outer handler special-cases committedGeneration !== undefined into a post_commit_failed warning plus succeeded_with_warnings. There's even an in-code comment saying a failed notification must not turn a committed mutation into a failed operation. status: 'failed' is only reachable when the commit never happened. I'd drop or re-evidence that half of the rationale — it's the part that makes this read as a correctness fix rather than a latency refactor.

Direction: aligned. Capability-gating a contract change is the established pattern on this surface, and the daemon-side mechanism isn't new: skipRefresh already exists on the controller options and POST /extensions/check-updates already passes it, so this extends an existing option instead of adding one. The new tag is registered unconditionally and is genuinely read by the Web Shell client, so it isn't a dead switch. CHANGELOG: no direct reference, but Extension management and the daemon protocol are actively developed areas.

Size: cross-package (packages/cli + packages/web-shell), so Stage 0 core paths apply. Production logic 56 lines (capabilities 4, extensions controller 12, extension routes 21, ExtensionsManagerPage 19); tests 462 lines; docs 52 lines. Well under the 500-production-line refactor hard block and under the 1000-line advisory, so no size gate fires. Noting it for maintainer awareness only because it's a refactor spanning two packages, which puts it under the Tier 2 confidence bar.

Approach: scope is tight and I couldn't find a smaller version of it. All seven skipRefresh: true additions land on activation routes only — global batch and singular, workspace batch and singular, the workspace DELETE clear, and the two legacy enable/disable routes. Install, update and uninstall keep calling globalReconciliationOptions() untouched, which matches the PR's claim. The deleted workspaceReconciliationOptions() helper had exactly two call sites, both rewritten here, so the deletion is complete rather than leaving dead code. The client side reuses the existing extensions.manage.refreshFailed key (present in both en and zh) and correctly extends the useCallback deps.

One question I couldn't settle from the diff, and it's the main thing I'd want answered before merging: the client-side remedy is narrower than the behaviour it replaces. PUT /extensions/activation and PUT /extensions/:extensionId/activation previously passed refreshRuntimes: () => workspaceRegistry.listAll(). The Web Shell now answers a global default change with workspaceByCwd(workspace.workspaceCwd).refreshExtensionRuntime(...), which resolves to POST /workspaces/:workspace/extensions/refresh → refreshRuntimes: [runtime]. So a global activation change made from the Web Shell immediately refreshes only the current workspace.

I did confirm the 30-second poller closes the gap — it filters listAll() by appliedGenerationByWorkspaceId !== generation, and onRuntimeReconciled never fires under skipRefresh, so every runtime stays pending and gets picked up. But that converts an immediate all-runtime guarantee into a ≤30s eventual one, and there's no client-callable "refresh all runtimes" operation to substitute. It's most pointed when disabling an extension globally: for up to 30 seconds it stays live in other workspaces' sessions while the UI already reports success.

Risk: no Stage 1e high-risk path match. The elevated item is the multi-workspace consistency window above, which is a product decision rather than a code defect.

Moving on to code review. 🔍

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 8b0e0e7944d4a8edb93d5005c849a8a32c63fc6c · re-run with @qwen-code /triage}

🩺 serve daemon A/B

Built the PR base vs this PR head 1f06d58, drove a fixed endpoint set against each, and diffed the JSON responses. Only fields that changed are shown.

capabilities

— Qwen Code · serve A/B

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 1f06d58. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

⚠️ One or more scenarios failed to render on this head, so this preview may be missing views — see the workflow run. The composites below are the scenarios that did render.

_{Full-resolution recordings (.webm) are attached to the workflow run.}

— Qwen Code · web-shell visuals

Code review

Read against 8b0e0e7944d4a8edb93d5005c849a8a32c63fc6c. My independent proposal before opening the diff was: keep the commit as the durability point, reuse the existing skipRefresh controller option on activation routes only, advertise a capability so old clients don't double-refresh, and have the client fire the existing refresh operation without awaiting it. That is essentially what this PR does, and it reuses the right primitives — skipRefresh is already on the controller options and already used by POST /extensions/check-updates, and the client reuses the existing extensions.manage.refreshFailed key, which is present in both the en and zh catalogs. So no parallel mechanism was invented. The routing is also correctly scoped: all seven skipRefresh: true additions are activation routes, and install/update/uninstall still call globalReconciliationOptions().

One blocker, one decision that needs a human, and some accuracy points.

Blocking — the new capability tag was not added to the integration test's baseline list

integration-tests/cli/qwen-serve-routes.test.ts has an advertises all baseline capabilities case that asserts the full tag array with toEqual, and extension_batch_activation_v2 sits in that hardcoded list. The new tag was never added, so the advertised array is one longer than expected and the check is red. This is the cause of the Integration Tests (no-AK, No Sandbox) failure below — not infra noise. The fix is a one-line insertion of 'extension_activation_explicit_refresh' in the same position it occupies in the registry.

I enumerated every hardcoded copy of that list to be sure this is the only miss, and it is: the registry itself, packages/cli/src/serve/server.test.ts, the tag list in docs/developers/qwen-serve-protocol.md, and the toContain assertion in workspace-qualified-extensions.test.ts were all updated. The PR body's local verification list covers route tests, controller tests, capability doc contract tests, Web Shell tests and the daemon server file — but not the integration suite, which is why this one slipped through.

Needs a product decision — a global activation change now refreshes only the current workspace

Covered in the gate comment, restating the code path concretely because it's the substance of the change. PUT /extensions/activation and PUT /extensions/:extensionId/activation previously passed refreshRuntimes: () => workspaceRegistry.listAll(). The Web Shell reaches the global route through scope === 'user' → setExtensionDefaultActivation(...), and then answers with workspaceByCwd(workspace.workspaceCwd).refreshExtensionRuntime(...), which is POST /workspaces/:workspace/extensions/refresh → refreshRuntimes: [runtime].

I traced the convergence path and it does hold: the poller filters workspaceRegistry.listAll() by appliedGenerationByWorkspaceId.get(runtime.workspaceId) !== generation, and because onRuntimeReconciled is only invoked on a successful reconcile, skipRefresh leaves every runtime pending, so the next 30-second tick refreshes all of them. Nothing is lost permanently.

What changes is the guarantee. Workspace-scoped activation is unaffected — it refreshed only [runtime] before and effectively does the same now. Global default activation goes from immediate-all-runtimes to ≤30s-eventual, and there is no client-callable operation that refreshes every runtime, so a client cannot opt back into the old behaviour. The sharpest case is a global disable: the UI reports success while the extension's commands, skills and MCP servers stay live in other workspaces' sessions for up to 30 seconds. Either submit the refresh per affected runtime, or state the window explicitly in the user-facing doc — right now eventual convergence is documented in the developer protocol and SDK pages, while docs/users/qwen-serve.md still reads as though the batch routes apply on completion.

Accuracy points

• The "refresh failures downgrade an otherwise successful policy commit" rationale is contradicted by the controller. Every refresh-failure path already yields succeeded_with_warnings; status: 'failed' is reachable only when committedGeneration === undefined, i.e. the commit never happened. Detail in the gate comment.
• "Wire schemas are unchanged" is true of the shape but not of the payload: activation operation results stop carrying refreshed and failed. I grepped the consumers and nothing in-repo reads those off the operation result — packages/sdk-typescript/src/daemon/ui/terminal.ts reads them off the extensions_changed broadcast event, and the bridge reads them off refreshExtensionsForAllSessions return values — so no code breaks here. A third-party client polling the operation would see the fields vanish, though, so the body is better off saying that plainly.
• The protocol doc tells clients to "wait for the activation operation to commit and then submit the independent runtime-refresh operation", naming both refresh routes. That is accurate for POST /workspaces/:workspace/extensions/refresh, which goes through sendOperation and returns a pollable handle, but the legacy POST /workspace/extensions/refresh is a plain synchronous handler returning the refresh result directly — not an operation. Worth disambiguating since both are named in the same sentence.

Non-blocking

docs/design/extension-management-v2.md picked up a broken reflow — the sentence now ends a line with "so only the winning" and continues "artifact commit activates a complete bundle." Purely cosmetic, and unrelated to the substance of that edit.

Everything else I checked held up: the fire-and-forget refresh is dispatched after load(true) and after the success message, with .catch attached, so the control unlocks as intended and a refresh failure still surfaces without an unhandled rejection; the useCallback dep array is correctly extended with activationRequiresExplicitRefresh and connection.clientId; and the tag count in 00-index.md was bumped 151→152 while correctly leaving "44 conditional tags" alone, since the new tag is registered unconditionally.

Flow after this change

sequenceDiagram
    participant P1 as Web Shell ExtensionsManagerPage
    participant P2 as activation route
    participant P3 as extensions controller
    participant P4 as Extension Store generation
    participant P5 as runtime bridge sessions
    participant P6 as 30s generation poller
    P1->>P2: activation change (global or workspace scope)
    P2->>P3: sendOperation with skipRefresh true
    P3->>P4: durable policy commit, generation bumps
    P3-->>P1: operation succeeded, no reconciling phase
    P1->>P5: refreshExtensionRuntime, current workspace only
    Note over P5: sessions in other workspaces are untouched here
    P6->>P4: read store generation
    P6->>P5: refresh every runtime whose applied generation lags

Testing

This is an unattended CI run, so nothing was built or executed here — the evidence below is this PR's own CI on the reviewed commit, read through the API. No tmux or real-scenario testing was driven.

Integration Tests (no-AK, No Sandbox) is red and it is PR-caused, not pre-existing infra noise: the assertion diff names extension_activation_explicit_refresh as the extra received element, which is exactly the tag this PR registers. Excerpt from the failing job (actions/runs/33834675931/job/100904690134):

FAIL  cli/qwen-serve-routes.test.ts > qwen serve — capabilities envelope > advertises all baseline capabilities
AssertionError: expected [ 'health', 'daemon_status', …(127) ] to deeply equal [ 'health', 'daemon_status', …(126) ]
- Expected
+ Received
@@ -76,10 +76,11 @@
     "workspace_skill_settings_batch_toggle",
     "extension_batch_activation_v2",
+   "extension_activation_explicit_refresh",
     "workspace_skill_manage",
❯ cli/qwen-serve-routes.test.ts:340:7

 Test Files  1 failed | 20 passed (21)
      Tests  1 failed | 173 passed (174)

The unit suite, lint, typecheck and the serve A/B job had not finished when this was fetched — Test (ubuntu-latest, Node 22.x), Lint & Static (ubuntu-latest, Node 22.x), Serve A/B, Capture web-shell visuals, OpenTUI no-flicker gate, Real daemon E2E / Java 11 and macos-latest / Java 21 were still in progress. Not verified: whether the changed unit tests, typecheck and lint pass, because those checks were still running. I did not poll for them.

Not verified: the author's local results quoted in the PR body (51 route tests, 14 controller tests, 2 capability doc contract tests, 4 Web Shell activation-refresh tests, the 1,176-test daemon server file, and three socket/timeout flakes passing on serial rerun). That is the author's claim, tested on macOS only, not evidence this review re-ran.

Final CI results for 8b0e0e7 (auto-updated by the triage finalize job after CI completed):

_{One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。}

Bot orchestration jobs (triage, review-pr, Classify PR, assign, label, authorize, the publish-* and precheck-pr pair, tmux-testing, verify) are omitted from the table — they are not PR CI.

Sandboxed verification would settle what CI cannot: @qwen-code /verify — that activation now reaches succeeded with no reconciling phase and no refreshed/failed fields is assertable from the route tests, but that the 30-second poller actually converges a second workspace after a global default change is not observable anywhere in this diff, and the suite would pass identically with the Web Shell refresh block deleted, since no test asserts another workspace catching up. @qwen-code /tmux covers the client half: that the activation control genuinely unlocks before the submitted refresh finishes, and that a daemon without the capability does not receive a duplicate refresh.

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 8b0e0e7944d4a8edb93d5005c849a8a32c63fc6c · re-run with @qwen-code /triage}

Confidence: 2/5 — the shape of this change is right and the diff is disciplined, but CI is red on a tag list the PR forgot to update, and one behaviour change is a product decision I shouldn't make from a diff.

My independent proposal before reading the code was almost exactly what this PR does: keep the commit as the durability point, reuse the controller's existing skipRefresh on activation routes only, gate the new contract behind a capability, and let the client fire the existing refresh without awaiting it. That agreement is a good sign about the direction, but it isn't the bar — matching my guess doesn't make it verified.

What I'd say honestly: this is a well-built PR. Fifty-six production lines accomplish the decoupling, the seven skipRefresh additions are all on activation routes with install/update/uninstall genuinely untouched, the deleted helper had exactly the two call sites the diff rewrites, and the client reuses an existing i18n key in both locales instead of adding one. The documentation work is unusually thorough — five doc files, the registered-tag count bumped, and the conditional-tag count correctly left alone because the new tag is unconditional. If I picked this up in six months I'd thank the author rather than curse them.

I'm still not approving, for three reasons in order of weight.

The integration check is red and this PR caused it. advertises all baseline capabilities asserts the full tag array with toEqual, and the new tag was never added to that hardcoded list. The assertion diff names extension_activation_explicit_refresh as the extra received element. That's a one-line fix, and I confirmed it's the only hardcoded copy that was missed — but I can't approve over a failing integration check that is pointing directly at this diff. The PR's local verification list covers the route, controller, doc-contract, Web Shell and daemon server suites, not the integration suite, which is exactly the gap.

The multi-workspace question is not mine to settle. Global default activation goes from refreshing every runtime immediately to refreshing the current workspace immediately and the rest within ~30 seconds via the poller. I traced the poller and convergence does hold, so nothing is lost permanently — but the guarantee genuinely weakens, there's no client-callable way to refresh all runtimes, and the sharpest case is a global disable where the UI reports success while the extension stays live in other workspaces' sessions for up to half a minute. That may well be the right tradeoff for the latency win. "May well be" is my judgment, not a verification, and this is a cross-package refactor over core paths, where the bar is certainty or escalation. I'm not certain, so I'm escalating rather than signing off. No maintainer resolved deterministically here — the PR carries no area label, has no human reviewer yet, and no maintainer handle is configured — so I'm not going to guess a login; this needs a human to weigh the latency win against the consistency window.

Half the stated motivation is wrong, and that changes what this PR is. Refresh failures already produce succeeded_with_warnings on all three paths, and the code carries an explicit comment that a failed notification must not turn a committed mutation into a failed operation. So the "downgrade an otherwise successful policy commit" rationale doesn't hold. If that was part of what made this feel urgent, the honest framing is a latency refactor with a documented consistency tradeoff — a different conversation, and one worth having on accurate premises.

What would move me to approve: the integration-test line; either a per-affected-runtime refresh on the global path or an explicit statement in docs/users/qwen-serve.md that a global activation change is applied to other workspaces within ~30 seconds rather than on completion; and correcting the downgrade claim in the description. The two doc imprecisions (the legacy refresh route isn't an operation handle; the design-doc reflow) are non-blocking and can ride along or not.

Note on evidence: the unit suite, lint, typecheck and serve A/B job were still running when I fetched CI, and I did not poll them — so those results are unknown to this review, not assumed green. The finalize workflow updates the CI table in the review comment once they settle. Nothing was built or executed here; the review is static plus this PR's own CI read through the API.

Requesting changes on the blocking item and deferring the design question to a maintainer. 🙏

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 8b0e0e7944d4a8edb93d5005c849a8a32c63fc6c · re-run with @qwen-code /triage}

Addressed every item from the requested-changes review on the current head:

• Added extension_activation_explicit_refresh to the strict integration capability list in eb0a9871af; the exact previously failing qwen-serve capability integration test passes against a freshly rebuilt bundle.
• Kept the product decision that Web Shell refreshes only its current workspace after global activation. The user-facing contract now states that other workspaces may retain the previous activation until the next 30-second generation-reconciler pass, with failures retried later (1f06d58d6e).
• Corrected the PR motivation to describe refresh warnings rather than implying the durable commit becomes failed.
• Distinguished the synchronous legacy refresh response from the asynchronous workspace-qualified refresh operation, and repaired the awkward design-document line break (1f06d58d6e).

There are no review threads. The requested-changes review is anchored to 8b0e0e7944; current head is 1f06d58d6e.